Skip to content

Merge E3SM/master into Omega/develop#371

Merged
mark-petersen merged 144 commits intodevelopfrom
master
Mar 26, 2026
Merged

Merge E3SM/master into Omega/develop#371
mark-petersen merged 144 commits intodevelopfrom
master

Conversation

@xylar
Copy link
Copy Markdown

@xylar xylar commented Mar 22, 2026

This brings in E3SM-Project#8165 with much-needed updates to PCD constants, among other changes.

Checklist

  • Testing
    • Add a comment to the PR titled Testing with the following:
      • Which machines CTest unit tests
        have been run on and indicate that are all passing.
      • The Polaris omega_pr test suite
        has passed, using the Polaris e3sm_submodules/Omega baseline
      • Document machine(s), compiler(s), and the build path(s) used for -p for both the baseline (Polaris e3sm_submodules/Omega) and the PR build
      • Indicate "All tests passed" or document failing tests

Copilot AI and others added 30 commits February 6, 2026 16:30
- Add transpose flag (m_transpose) to AtmosphereOutput class
- Add helper_fields map to store transposed layouts
- Read transpose parameter from YAML configuration
- Store dimensions in transposed order when transpose enabled
- Create helper fields for unique transposed layouts during init
- Transpose field data before writing to file in run()
- Apply transpose to both regular fields and avg count fields

Co-authored-by: AaronDonahue <11351562+AaronDonahue@users.noreply.github.com>
- Document transpose parameter in user guide (model_configuration.md)
- Add transpose to scorpio_output.hpp parameter list
- Explain transpose functionality in class documentation

Co-authored-by: AaronDonahue <11351562+AaronDonahue@users.noreply.github.com>
- Fix bug in helper field name construction (was calling transpose() twice)
- Add get_transposed_helper_name() function to reduce code duplication
- Use helper function consistently in all locations that access helper fields

Co-authored-by: AaronDonahue <11351562+AaronDonahue@users.noreply.github.com>
- Fix typo: 'specifyign' -> 'specifying' in scorpio_output.hpp
- Add comment explaining helper field naming convention

Co-authored-by: AaronDonahue <11351562+AaronDonahue@users.noreply.github.com>
Include data type in helper field names to support fields with
same layout but different data types (Real, int, float, double).
This ensures count fields (int) have separate helper fields from
regular fields (Real).

- Modified get_transposed_helper_name() to accept data type param
- Updated helper field FieldIdentifier to include data type
- Updated all call sites to pass data type when creating/looking up helpers

Co-authored-by: AaronDonahue <11351562+AaronDonahue@users.noreply.github.com>
Created io_transpose test that exercises transpose functionality with:
- INSTANT, MAX, MIN, and AVERAGE output types
- Multiple field layouts (1D, 2D, 3D)
- Different data types (via count fields for averaging)
- Verification that transposed data reads back correctly

Test follows same pattern as io_basic.cpp but with transpose enabled.

Co-authored-by: AaronDonahue <11351562+AaronDonahue@users.noreply.github.com>
Modified test to write both transposed and non-transposed output files,
then use the compare-nc-files script to verify they contain the same
data with transposed dimensions. This approach doesn't rely on
scorpio_input for reading transposed files.

Changes:
- Write function now takes transpose parameter
- Generate two files: io_transpose_N (normal) and io_transpose_T (transposed)
- Added CMake tests that use compare-nc-files to verify files match
- Tests run for all averaging types: INSTANT, MAX, MIN, AVERAGE

Co-authored-by: AaronDonahue <11351562+AaronDonahue@users.noreply.github.com>
Modified scorpio interface to skip the check that decomposed dimensions
must be the slowest striding ones when output is transposed. This allows
transposed output to work correctly even when the decomposed dimension
(e.g., ncol) is no longer at the end after transposition.

Changes:
- Add "transposed_output" attribute to variables in scorpio_output.cpp
- Check for this attribute in set_var_decomp() before enforcing stride check
- When attribute is present, skip the EKAT_REQUIRE check at line 797

Co-authored-by: AaronDonahue <11351562+AaronDonahue@users.noreply.github.com>
Modified transpose() function to skip the units compatibility check when
either the source or target field has Units::invalid(). This allows helper
fields (which use Units::invalid()) to be reused for transposing fields
with the same layout and data type but different units.

This matches the approach used in the original transposed_output branch
and allows the transpose functionality to work with fields that have
varying units.

Co-authored-by: AaronDonahue <11351562+AaronDonahue@users.noreply.github.com>
This commit adds the option "-a" or "--allow_transpose" to the utility
compare-nc-files.  When this is set, the comparison will continue even
if the two files have transposed dimensions.  Inside of the utility the
field arrays will be properly transposed so that both fields compared have
the same dimensions.

By default the the argument is "false" and will report an error when comparing
transposed fields.
Updated CMakeLists.txt to pass the -a (--allow_transpose) flag to the
compare-nc-files script when comparing transposed and non-transposed
output files. This flag was added in commit 0121704 by @AaronDonahue
to enable the script to handle transposed dimensions correctly.

Co-authored-by: AaronDonahue <11351562+AaronDonahue@users.noreply.github.com>
Modified set_var_decomp() to correctly calculate PIO decomposition offsets
for transposed variables. For transposed output, dimensions are reversed,
so we need to compute strides and offsets in the correct order.

Changes:
- Calculate strides based on actual dimension order in file
- Generate offsets accounting for transposed layout
- Apply correct stride for decomposed dimension
- Keep original logic for non-transposed output

This fixes the data corruption issue where transposed output had incorrect
values due to improper memory mapping.

Co-authored-by: AaronDonahue <11351562+AaronDonahue@users.noreply.github.com>
1. Corrected decomposition offset calculation for transposed output:
   - Calculate strides from fastest to slowest dimension (C order)
   - Generate offsets by iterating through all dimension combinations
   - Properly account for decomposed dimension position in layout
   - Fixes data ordering issue where values were written sequentially
     instead of in correct transposed positions

2. Optimize sync_to_host call:
   - Move f_out.sync_to_host() inside else block
   - Only sync f_out when not transposing (per @bartgol review)
   - Transposed path already syncs temp field after transpose

This resolves the issue where transposed files had correct shape but
incorrect data ordering.

Co-authored-by: AaronDonahue <11351562+AaronDonahue@users.noreply.github.com>
The previous implementation was iterating "for each DOF, then for each
non-decomposed dimension", but this doesn't match the actual memory
layout after transpose.

Fixed to generate offsets in proper C-order (last dimension fastest):
- Recursively iterate through dimensions in file order
- For decomposed dimension, iterate through local DOFs
- For non-decomposed dimensions, iterate through all indices
- This ensures offsets map local buffer positions (in memory order)
  to correct global file positions

This fixes the issue where normal[0,:72,0] was incorrectly mapping to
transposed[0,0:72,0] instead of properly distributing across the
transposed layout.

Co-authored-by: AaronDonahue <11351562+AaronDonahue@users.noreply.github.com>
Enhanced the unit test to directly verify that transposed data is correct
by reading back the written files and checking that data_N[col,lev] equals
data_T[lev,col] for all positions.

This validation would have caught the decomposition offset bug where data
was being written to wrong positions. The previous test only relied on the
external compare-nc-files script which was itself being fixed to handle
transposed data, so it couldn't catch bugs in the transpose implementation.

Changes:
- Added verify_transpose() function that reads both files using scorpio
- Verifies dimensions are correct
- Checks that dimension names are in correct order
- Validates actual data values at each position match when transposed
- This provides a direct in-test validation that will catch future bugs

Co-authored-by: AaronDonahue <11351562+AaronDonahue@users.noreply.github.com>
Removed code using non-existent scorpio functions (get_var_dims,
set_dim_decomp with 6 args). The detailed data validation is already
performed by the compare-nc-files Python script called in CMakeLists.txt,
which uses xarray to verify data_N[col,lev] == data_T[lev,col].

The simplified verify_transpose() now only checks that dimensions match
between files, which is sufficient for in-test validation since the
external script provides comprehensive value checking.

Co-authored-by: AaronDonahue <11351562+AaronDonahue@users.noreply.github.com>
This commit checks that attributes in stream elements are valid and
produces a useful error message if not.  Previously, execution would
die without error.

Authored by Copilot
This commit is an extension of previous, adding validation for child
tags.

Authored by Copilot
In the unusual situation where MPI is not included, this declaration
needs to be outside the MPI ifdef.
The xarray-based rewrite introduced a bug where tolerance was not being
applied when comparing values. The script was using exact equality check
instead of applying the tolerance parameter.

Fixed by:
- Computing absolute difference between arrays
- Applying tolerance check: diff > (tol + tol * abs(rhs))
- Only reporting failure when values exceed tolerance
- Using same tolerance logic as original netCDF4-based implementation

This fixes the cld_frac_net_standalone_cpp_vs_py test failure where
small floating-point differences (3.57e-07) were being flagged even
though they were within the specified tolerance (1e-6).

Co-authored-by: AaronDonahue <11351562+AaronDonahue@users.noreply.github.com>
…t_stopf

Co-authored-by: ndkeen <5684727+ndkeen@users.noreply.github.com>
bartgol and others added 14 commits March 23, 2026 22:04
The mask field name must account for the vector dimension extent
to avoid using the same mask for two different vector fields
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Store different mask fields for different vector layouts.

[BFB]
…8125)

MPAS stream syntax checks

This PR adds syntax checks to MPAS stream elements and returns a useful
error if unexpected tags are found. Previously, unexpected tags (e.g.,
typos) would result in model termination with no error information
provided to the user. Fixes one issue in the default streams.ocean file
uncovered during testing.

PR also includes a small MPAS framework fix to the logging module when
MPI is not active.

[BFB]
Fix ICE with oneapi-ifx v2025.2.0 on Chrysalis

[BFB]
The alias method is actually a clone, there is no aliasing
Implement Transposed Output for EAMxx
    
Based on the stale branch aarondonahue/transposed_output, implementing
feature to allow users to request transposed output from the eamxx
component.
    
Summary
    
This PR adds the ability for users to request transposed output in
EAMxx by setting transpose: true in their output YAML configuration
files. When enabled, all field dimensions are reversed in the output
NetCDF files (e.g., (ncol, nlev) becomes (nlev, ncol)).

[BFB]
Simplify creation of new identifiers from old ones by
modifying only a handful of the properties.

[BFB]
This pull request passes HLM biophysical landuse variables into fates to by output as diagnostic variables along FATES dimensions.
 Thefates_cold_luh2 test mod is updated to include these new diagnostic variables.
This also updates the fates_cold_allvars testmod to include new history variables added with NGEET/fates#1485.

This PR is the ELM-side code changes associated with FATES PR NGEET/fates#1536.

[non-BFB] for FATES due to field lists
Extend `atmchange -r` to accept an optional node name,
resetting only the changes targeting that node and its descendants.

[BFB]
Add Full Wave-Sea Ice coupling for wave-enabled E3SM simulations.
This PR is bit-for-bit with standard E3SM configurations (no active waves), 
it only generates climate changing impacts if an active wave compset is used.

This PR is the last of all PRs for the LRW (Low Res Wave) configuration of E3SM V3. 
This PR enables Full two-way coupling between WW3 and MPAS-SI.

This PR adds the following new features to E3SM:
  - Floe Size Distribution in MPAS-SI
  - Floe Breaking and re-distribution of floe size distribution due to waves.

Note: This PR requires the use of a new WW3 submodule that allow 
wave-ice coupling, as follows:
WW3:
E3SM-Project/WW3#7

[BFB] when no wave model is used.
[NML]
@mark-petersen mark-petersen self-requested a review March 26, 2026 16:32
Copy link
Copy Markdown
Collaborator

@mark-petersen mark-petersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested on Frontier CPU and GPU and Perlmutter CPU, on the head of omega/develop and then with 2c41b24 merged in from master. It passed all cTests. I had trouble with Perlmutter GPU on the head of develop, but that is unrelated to this update.

I was able to merge without any conflicts:

$ git reset --hard omega/develop
HEAD is now at 29b403dc8e Merge pull request #369 from hyungyukang/hkang/function-name-change
$ git merge 2c41b2401ef705874895eeff977682b7670c1b96
Merge made by the 'ort' strategy.

so I don't understand what the merge conflict on the PR is. ort stands for "Ostensibly Recursive's Twin". Huh. Well, maybe we need to merge at the command line with ort and git push rather than with the github button.

@sbrus89
Copy link
Copy Markdown
Collaborator

sbrus89 commented Mar 26, 2026

I think the issue is that #369 updated ekat (as well as scorpio and cime). We'll need a PR to revert those changes similar to #360.

@sbrus89
Copy link
Copy Markdown
Collaborator

sbrus89 commented Mar 26, 2026

Thanks @katsmith133!

@mark-petersen
Copy link
Copy Markdown
Collaborator

OK, now that #379 is merged, the button at the top of this PR changed to 'Ready to Merge'. On PM GPU we need to add the cmake flag -DCPPTRACE_UNWIND_WITH_NOTHING=ON to complete the cmake command. It then compiles and passes all cTests.

Note, this PR does not alter any of the submodules used by Omega, which are these:

components/omega/external/GSW-C 
components/omega/external/yaml-cpp 
externals/ekat 
externals/scorpio 
cime 
externals/cpptrace

@xylar
Copy link
Copy Markdown
Author

xylar commented Mar 26, 2026

If now is a good time to merge this, someone should do so.

@katsmith133
Copy link
Copy Markdown

Testing (note: this testing is post PR #379 being merged)

CTest unit tests:

  • Machine: PM-CPU and PM-GPU
  • Compiler: gnu and gnugpu
  • Build type: Release
  • Result: All tests passed, but PM-GPU still requires -DCPPTRACE_UNWIND_WITH_NOTHING=ON config flag

Note (mostly to myself): this PM-GPU issue is documented in #364 which seems to appear between PR #360 and #361 (ie. I tested with #360 and it worked and then tested with #361 and it did not work, at least not without the -DCPPTRACE_UNWIND_WITH_NOTHING=ON flag). Looks like according to PR #374, @xylar suggests that the fix needs to be made in E3SM-Project/E3SM instead of in Omega.

@xylar
Copy link
Copy Markdown
Author

xylar commented Mar 26, 2026

@xylar suggests that the fix needs to be made in E3SM-Project/E3SM instead of in Omega.

I originally felt that way. I still think updating submodules that Omega doesn't own is a bad move, but that particular submodule isn't in E3SM yet so maybe we can punt on cleaning that up for now, with the caveat that it may mean a bunch of revert commits in the future...

@mark-petersen mark-petersen merged commit e85ea7e into E3SM-Project:develop Mar 26, 2026
@xylar
Copy link
Copy Markdown
Author

xylar commented Mar 26, 2026

Thanks @mark-petersen and @katsmith133! I'm going to go ahead and merge.

@xylar
Copy link
Copy Markdown
Author

xylar commented Mar 26, 2026

@mark-petersen beat me to it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.